BLD/DEV: use meson-python, add a basic Pixi workspace#373
Conversation
ev-br
left a comment
There was a problem hiding this comment.
Let's maybe revive this if you're interested @lucascolley ? Sorry it took me long to get back to this.
I trust you on anything pixi. If dynamic versioning gets in the way, I'd be happy to see the back of it, static is indeed much nicer.
What needs to be done if we change the versioning:
- purge any mentions of
setuptools_scmandtool.setuptools.dynamicfrompyproject.toml - update the releasing docs to mention that the version needs to be bumped in both
__init__.pyandpyproject.tomlit currently says "update the version in__init__.py" - cut a dummy release locally (
python -m build . && twine check --strict dist/ - make a dummy TestPyPI release to verify it all works
6c2a523 to
82e4abe
Compare
❯ pixi exec -w python-build python -m build .
...
❯ pixi exec twine check --strict dist/*.tar.gz dist/*.whl
Checking dist/array_api_compat-1.13.0-py3-none-any.whl: PASSED
Checking dist/array_api_compat-1.13.0.tar.gz: PASSED
do you want me to do this? |
|
I implemented the switch from setuptools to meson-python |
ev-br
left a comment
There was a problem hiding this comment.
Agreed that the change to meson-python is nice.
I actually wonder about vendoring scenarios. When e.g. SciPy vendors array-api-compat, it has to list the sources in the meson.build file. Now that array-api-compat has its meson.build file with the listing, can scipy reuse it? This is not blocking this PR of course, am just thinking out loud.
make a dummy TestPyPI release to verify it all works
Thinking about it, to test trusted publishing it needs to be done from main/tag etc. Presumably trusted publishing is decoupled enough, so that it's more work than justified now. So checking that a release wheel looks reasonable locally is probably sufficient.
|
Yes, this is the exact context within which I started this work: https://github.com/scipy/scipy/blob/main/meson.build#L216-L246 |
|
Tried test-driving it with my usual workflow: In fact, non-editable installs fail in the same way as well. What gives? |
|
Hmm the problem is reproducible even in the default Pixi environment via |
93ba85a to
2470147
Compare
| install_symlink( | ||
| '_compat', | ||
| install_dir: 'array_api_compat/vendor_test/vendored', | ||
| pointing_to: '../../', | ||
| ) |
There was a problem hiding this comment.
The problem was that I had a dodgy workaround for the symlink. Hopefully now this works.
There was a problem hiding this comment.
@rgommers can you see what I'm doing wrong with the symlink here? Logs like https://github.com/data-apis/array-api-compat/actions/runs/23195083314/job/67401163424?pr=373 seem to show that it is broken.
I've tried poking around with meson introspect but I can't figure it out.
There was a problem hiding this comment.
Symlinks aren't supported in wheels, so this looks unfamiliar and is probably not what you want.
It is possible to have a symlink in the source tree, but it gets replaced by a copy in an sdist/wheel.
There was a problem hiding this comment.
9120e06 (this PR) gets rid of the symlink. However, this does not seem to work for editable builds. Is there something in Meson's Python module that can mimic configure_file(copy: true) like this?
For context, it seems like pointing to the same file twice with py.install_sources means that it is only installed once.
There was a problem hiding this comment.
despite my sunk-cost into making this work for wheels only, we should probably just give up on the symlink and use a Pixi task to populate the directory at test-time, like https://github.com/data-apis/array-api-extra/blob/1c5bb9d65f75132196650de2ca966e2fc36ac15c/pyproject.toml#L146. I think we are pushing meson-python slightly past its limits by trying to mimic the symlink.
I had hoped that we could merge this without me having to convert CI over to using Pixi yet, but maybe not...
There was a problem hiding this comment.
I'm a little confused here about what triggered all this, because there's a lot going on in this PR:
- The move to
src/layout, if desired, might be best to land standalone, together with a commit to not affectgit blame - The vendor test is new, and is what seems to be the motivation for the symlink-like thing. Can't this be done inside the test itself, rather than in the source tree?
- Switching over to
meson-python, nice (for SciPy/scikit-learn and the vendoring use case) - Adding a Pixi config
There was a problem hiding this comment.
- The vendor test is new, and is what seems to be the motivation for the symlink-like thing. Can't this be done inside the test itself, rather than in the source tree?
the vendor test is not new, you can see the symlink at https://github.com/data-apis/array-api-compat/tree/main/vendor_test/vendored. The challenge I was trying to get through was dealing with the symlink in meson-python. It would be nicer to just get rid of the symlink and copy things over at test-time, like https://github.com/data-apis/array-api-extra/blob/231b8a50d9baa22ccebdfee571472f0ce46a8a29/pyproject.toml#L145-L146.
There was a problem hiding this comment.
Ah never noticed (or, no longer remembering) - it's missing from the sdist, which is unsurprising but not ideal.
It would be nicer to just get rid of the symlink and copy things over at test-time
agreed
c47c034 to
8f4f87e
Compare
|
@ev-br please could you let me know if you have any remaining concerns? thanks! |
ev-br
left a comment
There was a problem hiding this comment.
A couple of questions below.
Other than that:
- pixi changes I happily defer to you Lucas :-).
- dynamic versioning -> static versioning change is great, let's rid of it.
- meson is a bit of an overkill for a pure python package --- I'd use flit for a pure-python package with static versioning, but OK if we want to use meson throughout;
srclayout: is it needed (why?) or is it just because why not (also why though?)- could you double-check what scipy and other vendorers need to do to adapt?
| def test_vendoring_dask(): | ||
| pytest.importorskip("dask") | ||
| from vendor_test import uses_dask | ||
| from . import uses_dask |
There was a problem hiding this comment.
Could you explain this and related changes please?
There was a problem hiding this comment.
test_vendoring was moved from the tests directory to the vendor_tests directory, and this line is updated with the appropriate relative import. This matches what works well for us at https://github.com/data-apis/array-api-extra/tree/main/vendor_tests.
| @@ -1,3 +1,6 @@ | |||
| pixi.lock | |||
| vendor_tests/array_api_compat | |||
There was a problem hiding this comment.
Could you explain these two?
-
ignoring
pixi.lock: naively, I'd expect that if some dependency breaks, we'd want to bisectpixi.lockupdates. From experience, is this not a useful workflow? -
Never saw any issues with
vendor_tests/array_api_compat(or the very directory for that matter), why do we want to ignore it now?
There was a problem hiding this comment.
I'm happy to commit pixi.lock. Without committing it, each individual contributor may have different environments on their machine, although they will all still be compatible with pixi.toml.
I was making the minimal change in case there were objections to committing the lock file, but if there aren't, then great, let's commit it.
There was a problem hiding this comment.
- Never saw any issues with
vendor_tests/array_api_compat(or the very directory for that matter), why do we want to ignore it now?
The vendoring tests are now changed from the previous symlink at vendor_test/vendored/_compat to a cp command that populates this newly gitignored directory. We discussed above that we wanted to get rid of the symlink.
There is a Pixi task for this cp command that runs automatically with pixi run tests-vendor, and it is currently hardcoded in CI. I've marked converting CI to Pixi for a follow-up.
I agree that in general meson-python is overkill for a pure Python package, but in this specific case where we want to make it easily vendor-able by projects using meson-python, it makes sense
I think I originally introduced it because it kept
The work is already done for SciPy, this just enables us to delete https://github.com/scipy/scipy/blob/main/subprojects/array_api_compat/meson.build. Other vendorers can copy SciPy's approach. |
Clearly plenty more work to do to get this up to par with array-api-extra, but I think that merging something like this would be a good start.
Follow-ups: